Skip to content

Conversation

@zanesq
Copy link
Collaborator

@zanesq zanesq commented Jul 28, 2025

Ensure adding/removing extensions refreshes extensions list

Also added support for renaming an extension without adding a new one.

fixes 3680

});
await reloadConfig();
// Force refresh extensions list to ensure UI is updated
setExtensionsList([]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused on the need for this one. Doesn't:

await getExtensions(true) in fetch extensions already do a force reload?

The logic for loading extensions is very fiddly and that's my fault. I just want to be sure of the need for more imperative things to get extensions to reload before we merge.

Instead of manually clearing extensionsList state with setExtensionsList([]),
we now properly call getExtensions(true) which forces a refresh through
the existing API. This addresses the concern about imperative state
manipulation and uses the established refresh pattern.

- Extracted refreshExtensions helper to avoid circular dependencies
- Both addExtension and removeExtension now properly refresh the list
- Maintains the same functionality but with cleaner architecture
@zanesq
Copy link
Collaborator Author

zanesq commented Jul 31, 2025

I've addressed your concern about the imperative state clearing approach. The fix now uses a cleaner pattern:

What changed:

  • Removed the manual setExtensionsList([]) calls
  • Created a dedicated refreshExtensions helper function that handles the API call and state update
  • Both addExtension and removeExtension now call await refreshExtensions() after successful operations
  • getExtensions(forceRefresh) now delegates to refreshExtensions when a refresh is needed

Why this is better:

  • No more imperative state manipulation
  • Uses the established async refresh pattern consistently
  • Cleaner separation of concerns
  • Avoids circular dependency issues
  • More predictable and maintainable

The functionality remains the same - the extensions list will properly refresh after add/remove operations - but now it's done through the proper async refresh mechanism rather than manually clearing state.

// Check if the sanitized name has changed
const nameChanged = sanitizedOriginalName && sanitizedOriginalName !== sanitizedNewName;

if (nameChanged) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handling this looks good as I know it was something missed before, but some new tests for it would be good! Either in this or a followup

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good will follow up with tests now that we have vitest!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexhancock
Copy link
Collaborator

I've addressed your concern about the imperative state clearing approach. The fix now uses a cleaner pattern:

What changed:

  • Removed the manual setExtensionsList([]) calls
  • Created a dedicated refreshExtensions helper function that handles the API call and state update
  • Both addExtension and removeExtension now call await refreshExtensions() after successful operations
  • getExtensions(forceRefresh) now delegates to refreshExtensions when a refresh is needed

Why this is better:

  • No more imperative state manipulation
  • Uses the established async refresh pattern consistently
  • Cleaner separation of concerns
  • Avoids circular dependency issues
  • More predictable and maintainable

The functionality remains the same - the extensions list will properly refresh after add/remove operations - but now it's done through the proper async refresh mechanism rather than manually clearing state.

Yes, new implementation looks better!

@zanesq zanesq merged commit c9ae9ba into main Jul 31, 2025
9 checks passed
@zanesq zanesq deleted the zane/extension-refresh branch July 31, 2025 19:17
michaelneale added a commit that referenced this pull request Jul 31, 2025
* main:
  Increase req body limit (#2965)
  Stable goose info -v (#3760)
  Speed up app initialization and improve refresh crashing (#3717)
  docs: consolidate search session content, doc import recipe (#3759)
  Improve power save blocker mechanism (#3698)
  Ensure adding/removing extensions refreshes extensions list (#3695)
  Env parsing for primitive types (#3706)
  Autocompact + One Shot Summarization algorithm (#3559)
  fix: initial prompt not filled in after accepting new recipe (#3637)
  fix not being able to click on searchbar buttons in chat (#3723)
  center session summary modal description text (#3737)
  Persist first message to local history in case of failure or cancellation (#3744)
  Make the client more secure (#3742)
  feat: Allow configuring hints filename(s) (#3269)
  Add support for mouse back nav button to Settings screen (#3195)
  chore: Remove the wrong tailwind package (#3754)
  chore: fix typo in desktop readme for goosed (#3752)
  feat: upgrade rmcp (#3738)
  feat: allow users view and edit their non-secret config's (#3005)
  fix: View extensions link (#3751)
michaelneale added a commit that referenced this pull request Aug 1, 2025
* main:
  fix: don't return full shell output when very large (#3750)
  fix: cli tool logging (#3749)
  Increase req body limit (#2965)
  Stable goose info -v (#3760)
  Speed up app initialization and improve refresh crashing (#3717)
  docs: consolidate search session content, doc import recipe (#3759)
  Improve power save blocker mechanism (#3698)
  Ensure adding/removing extensions refreshes extensions list (#3695)
  Env parsing for primitive types (#3706)
  Autocompact + One Shot Summarization algorithm (#3559)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI Bug: Extensions List Stale After Installation

3 participants